ADD: UInt256 implementation of evm's op MOD by thomas-quadratic · Pull Request #9188 · besu-eth/besu

thomas-quadratic · 2025-09-18T05:30:54Z

PR description

This PR implements the MOD operation in the EVM using a custom UInt256 class meant to replace BigInteger for optimised arithmetics.

The UInt256 class should grow to contain other arithmetic operations, but as of now contains mod, computing the remainder of a division by the modulus. It should run about 45% faster than with BigIntegers (depending on integers' sizes). More optimisations should come later as well, but this should be already useful for early release.

Fixed Issue(s)

I did not find an opened issue for this task.

Thanks for sending a pull request! Have you done the following?

Checked out our contribution guidelines?
Considered documentation and added the doc-change-required label to this PR if updates are required.
Considered the changelog and included an update if required.
For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

spotless: ./gradlew spotlessApply
unit tests: ./gradlew build
acceptance tests: ./gradlew acceptanceTest
integration tests: ./gradlew integrationTest
reference tests: ./gradlew ethereum:referenceTests:referenceTests
hive tests: Engine or other RPCs modified?

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

ahamlat · 2025-09-24T09:39:05Z

I propose adding a "fully random" case to the microbenchmarks. While it isn’t algorithmically meaningful, it can reveal real-world performance effects (branch prediction, cache behavior, allocation patterns) under heterogeneous inputs :
main...ahamlat:besu:feature/uint256-op-modulus-full-random#diff-bb4eb8b2c43632374a61d883bf5c0828013b6dc076898c846c58badba2ac3e22R53

thomas-quadratic · 2025-09-24T10:14:07Z

I propose adding a "fully random" case to the microbenchmarks.
Yes, it was left in and separated the other tests in another benchmark class, but the way you add it in your PR is preferable in my opinion.
I will update the PR with your random case.

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

ahamlat · 2025-09-24T14:21:33Z

@thomas-quadratic Could you add this shortcut improvement
main...ahamlat:besu:feature/uint256-op-modulus-full-random#diff-5278aa622ca5723b7a5661875be3502a1f215537a805db99ed83aa1af51f7056R54

This will resolve the only regression I see from the benchmarks

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

siladu

I converted Nethermind's UInt256 tests to Java and found and fixed some bugs (bug fixes mentioned in suggestions): 62c7946 (#9264)
(to compare when the base branch changes https://github.com/hyperledger/besu/compare/thomas-quadratic:feature/uint256-op-modulus..siladu:uint256-op-modulus-parameterised-tests)

There was a recent regression in mod durign your last force push, before yesterday it was passing the parameterised testMod.

There are bugs in shiftLeft and shiftRight but these shouldn't get hit by valid mod inputs...however since they are public methods probably makes sense to fix them?

Finally, do we need a division by zero test for the mod methods?

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

lu-pinto · 2025-10-03T12:26:20Z

Do you need any of these methods right now just for getting the opcodes going?

intValue
longValue
fromLong
fromInt

If not I would recommend removing them to not overcomplicate right now. They create API consistency problems and with a sign field approach they will require more work to get right and can be added later.

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

lu-pinto · 2025-10-03T12:46:55Z

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

+   * @param sign padding with 0s or 1s whether sign is non-negative.
+   * @return HexString
+   */
+  public String toString(final int sign) {


also can't see why you need to print UInt256 with a sign - plz remove

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

siladu

I've ported over some more tests for SMod and pushed to #9264

https://github.com/hyperledger/besu/compare/thomas-quadratic:feature/uint256-op-modulus..siladu:uint256-op-modulus-parameterised-tests

Tests are passing ✅

Not in the PR, but I also tried porting over some of geth's uint256 tests and these were passing except for SMod. The failures were related to how UInt256 were constructed which is what led me to the comment about the signedMod input assumptions.

evm/src/main/java/org/hyperledger/besu/evm/operation/SModOperation.java

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>

siladu · 2025-10-08T10:58:13Z

@thomas-quadratic I reran the tests from #9264 against your latest commits and testAdd and testLeftShift are failing now. Doesn't seem critical since they aren't actually used, but maybe we should either remove the public methods or fix...not sure these will be included in the fuzzing.

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java

lu-pinto · 2025-10-09T13:45:35Z

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java

+      Bytes32 expected =
+          BigInteger.ZERO.compareTo(big_modulus) == 0
+              ? Bytes32.ZERO
+              : bigIntTo32B(big_number.mod(big_modulus));
+      assertThat(remainder).isEqualTo(expected);
+    }
+  }


Suggested change

Bytes32 expected =

BigInteger.ZERO.compareTo(big_modulus) == 0

? Bytes32.ZERO

: bigIntTo32B(big_number.mod(big_modulus));

assertThat(remainder).isEqualTo(expected);

}

}

BigInteger expected = BigInteger.ZERO;

if (BigInteger.ZERO.compareTo(big_modulus) != 0) {

expected = big_number.mod(big_modulus);

}

assertThat(bytesToBigInt(remainder, 1)).isEqualTo(expected);

}

}

lu-pinto · 2025-10-09T13:46:01Z

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java

+    Bytes32 expected =
+        BigInteger.ZERO.compareTo(mbig) == 0 ? Bytes32.ZERO : bigIntTo32B(xbig.add(ybig).mod(mbig));
+    assertThat(remainder).isEqualTo(expected);
+  }


Suggested change

Bytes32 expected =

BigInteger.ZERO.compareTo(mbig) == 0 ? Bytes32.ZERO : bigIntTo32B(xbig.add(ybig).mod(mbig));

assertThat(remainder).isEqualTo(expected);

}

BigInteger expected = BigInteger.ZERO;

if (BigInteger.ZERO.compareTo(mbig) != 0) {

expected = xbig.add(ybig).mod(mbig);

}

assertThat(bytesToBigInt(remainder, 1)).isEqualTo(expected);

}

lu-pinto · 2025-10-09T13:46:58Z

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java

+      Bytes32 expected =
+          BigInteger.ZERO.compareTo(cInt) == 0
+              ? Bytes32.ZERO
+              : bigIntTo32B(aInt.multiply(bInt).mod(cInt));
+      assertThat(remainder).isEqualTo(expected);
+    }
+  }


Suggested change

Bytes32 expected =

BigInteger.ZERO.compareTo(cInt) == 0

? Bytes32.ZERO

: bigIntTo32B(aInt.multiply(bInt).mod(cInt));

assertThat(remainder).isEqualTo(expected);

}

}

BigInteger expected = BigInteger.ZERO;

if (BigInteger.ZERO.compareTo(cInt) != 0) {

expected = aInt.multiply(bInt).mod(cInt);

}

assertThat(bytesToBigInt(remainder, 1)).isEqualTo(expected);

}

}

lu-pinto · 2025-10-09T13:51:47Z

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java

+  private Bytes32 bigIntTo32B(final BigInteger x) {
+    byte[] a = x.toByteArray();
+    if (a.length > 32) return Bytes32.wrap(a, a.length - 32);
+    return Bytes32.leftPad(Bytes.wrap(a));
+  }
+
+  private Bytes32 bigIntToSigned32B(final BigInteger x) {
+    if (x.signum() >= 0) return bigIntTo32B(x);
+    byte[] a = new byte[32];
+    Arrays.fill(a, (byte) 0xFF);
+    byte[] b = x.toByteArray();
+    System.arraycopy(b, 0, a, 32 - b.length, b.length);
+    if (a.length > 32) return Bytes32.wrap(a, a.length - 32);
+    return Bytes32.leftPad(Bytes.wrap(a));
+  }


I think it is much easier to convert Bytes -> BigInteger than the reverse because BigInteger has better facilities to negate numbers and padding comes for free since BigInteger removes zeros anyway. You just need to figure out the sign.

So I would recommend removing the code here and replace it with:

private static BigInteger bytesToBigInt(final Bytes bytes, final int sign) { // bytes can be shorter, so it's treated as left padded with zeros if (bytes.size() < 32) { return new BigInteger(1, bytes.toArrayUnsafe()); } return sign < 0 ? new BigInteger(bytes.toArrayUnsafe()) : new BigInteger(1, bytes.toArrayUnsafe()); }

Have been running tests indefinitely for an hour with the suggestions above and no failures.

siladu · 2025-10-10T03:09:15Z

Fix for add here thomas-quadratic#3

- shiftRight: wrong limb/index math for edge inputs - shiftLeft: array index out of bounds on all-zero slice - mod: knuth remainder misnormalizes, wrong indices/carries - mod (1-limb): uses signed %, not unsigned bigInteger is the reference model. failures are defects in evm/UInt256.java. Signed-off-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>

Signed-off-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>

Use Bytes32.leftPad to normalize inputs before constructing BigIntegers and UInt256 values in property_signedMod_matchesEvmSemantics. Avoids sign-extension and length mismatch issues for shorter byte arrays. Signed-off-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>

The shiftLeft operation did not make sure to provision enough space for the result of shiftLeftInto. Now it does so, but it is somewhat wasteful at the moment, as max 8 limbs are going to be kept. This should be optimised later on. Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>

siladu · 2025-10-13T06:26:50Z

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

+  private final int[] limbs;
+  private final int length;
+
+  int[] limbs() {


nit: add @VisibleForTesting

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

siladu · 2025-10-13T06:29:02Z

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

+  // --------------------------------------------------------------------------
+
+  UInt256(final int[] limbs, final int length) {
+    // Unchecked length: assumes limbs have length == N_LIMBS


Is it an optimisation to leave unchecked?

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>

thomas-quadratic · 2025-10-13T15:08:36Z

Thanks everyone for your reviews, I think the API is stronger now.
We will deploy this PR, plus some of yours on top, and we will continue investigating optimisations, like long limbs, but also on int limbs.

* ADD: UInt256 implementation of evm's op MOD Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com> Signed-off-by: Nikos Baxevanis <nikos.baxevanis@gmail.com> Co-authored-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>

* ADD: UInt256 implementation of evm's op MOD Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com> Signed-off-by: Nikos Baxevanis <nikos.baxevanis@gmail.com> Co-authored-by: Nikos Baxevanis <nikos.baxevanis@gmail.com> Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>

matkt marked this pull request as draft September 19, 2025 05:28

thomas-quadratic force-pushed the feature/uint256-op-modulus branch from c2876f7 to 83afdd4 Compare September 22, 2025 12:43

matkt reviewed Sep 22, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Show resolved Hide resolved

macfarla added the performance label Sep 23, 2025

thomas-quadratic force-pushed the feature/uint256-op-modulus branch from 04317b5 to 4138151 Compare September 24, 2025 05:36

MariusVanDerWijden reviewed Sep 24, 2025

View reviewed changes

thomas-quadratic force-pushed the feature/uint256-op-modulus branch 2 times, most recently from fe159ea to 21a3e6a Compare September 30, 2025 17:34

ahamlat reviewed Oct 1, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

siladu mentioned this pull request Oct 2, 2025

Add parameterised tests for UInt256 #9264

Closed

siladu reviewed Oct 2, 2025

View reviewed changes

lu-pinto reviewed Oct 2, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 2, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 2, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 3, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 3, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 3, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 3, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 3, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 3, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 4, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

siladu reviewed Oct 6, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/operation/SModOperation.java Show resolved Hide resolved

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Show resolved Hide resolved

ahamlat reviewed Oct 6, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Show resolved Hide resolved

lu-pinto reviewed Oct 6, 2025

View reviewed changes

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 7, 2025

View reviewed changes

evm/src/main/java/org/hyperledger/besu/evm/UInt256.java Outdated Show resolved Hide resolved

thomas-quadratic force-pushed the feature/uint256-op-modulus branch from af9125a to 14774b9 Compare October 7, 2025 09:20

TEST: add fuzzing test case

7fb232e

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>

lu-pinto reviewed Oct 9, 2025

View reviewed changes

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java Outdated Show resolved Hide resolved

lu-pinto reviewed Oct 9, 2025

View reviewed changes

evm/src/test/java/org/hyperledger/besu/evm/UInt256Test.java Show resolved Hide resolved

lu-pinto reviewed Oct 9, 2025

View reviewed changes

thomas-quadratic force-pushed the feature/uint256-op-modulus branch from 6c36d5c to 0cdb13e Compare October 10, 2025 08:08

moodmosaic and others added 4 commits October 10, 2025 11:21

Run ./gradlew :evm:spotlessApply

13caf1f

Signed-off-by: Nikos Baxevanis <nikos.baxevanis@gmail.com>

thomas-quadratic force-pushed the feature/uint256-op-modulus branch from 0cdb13e to 98c67b0 Compare October 10, 2025 09:54

thomas-quadratic added 2 commits October 10, 2025 23:58

FIX: remove unused methods add, mul, shifts

3667423

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>

ENH: reincrease the sample size in unit tests to 300

7c5b000

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>

siladu reviewed Oct 13, 2025

View reviewed changes

FIX: removing deadcode and some javadocs

f2a7157

Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>

thomas-quadratic marked this pull request as ready for review October 13, 2025 15:09

Merge branch 'main' into feature/uint256-op-modulus

2aa7779

ahamlat approved these changes Oct 13, 2025

View reviewed changes

ahamlat merged commit 0db2a0a into besu-eth:main Oct 13, 2025
46 checks passed

ahamlat mentioned this pull request Oct 23, 2025

Address performance improvements comments on UInt246 #9355

Closed

This was referenced Nov 20, 2025

Implement EVM arithmetic operations using the new UInt256 implementation #9472

Closed

Optimize performances of AND, OR, XOR and NOT opcodes #9489

Merged

-    Bytes32 expected =
-        BigInteger.ZERO.compareTo(mbig) == 0 ? Bytes32.ZERO : bigIntTo32B(xbig.add(ybig).mod(mbig));
-    assertThat(remainder).isEqualTo(expected);
-  }
+    BigInteger expected = BigInteger.ZERO;
+    if (BigInteger.ZERO.compareTo(mbig) != 0) {
+      expected = xbig.add(ybig).mod(mbig);
+    }
+    assertThat(bytesToBigInt(remainder, 1)).isEqualTo(expected);
+  }

Conversation

thomas-quadratic commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

Locally, you can run these tests to catch failures early:

Uh oh!

Uh oh!

ahamlat commented Sep 24, 2025

Uh oh!

thomas-quadratic commented Sep 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahamlat commented Sep 24, 2025

Uh oh!

Uh oh!

Uh oh!

siladu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lu-pinto commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lu-pinto Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

siladu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

siladu commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lu-pinto Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

lu-pinto Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

lu-pinto Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

lu-pinto Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

siladu commented Oct 10, 2025

Uh oh!

siladu Oct 13, 2025

Choose a reason for hiding this comment

thomas-quadratic commented Sep 18, 2025 •

edited

Loading

siladu left a comment •

edited

Loading

lu-pinto commented Oct 3, 2025 •

edited

Loading

lu-pinto Oct 3, 2025 •

edited

Loading

siladu commented Oct 8, 2025 •

edited

Loading

lu-pinto Oct 9, 2025 •

edited

Loading